Skip to content

fix(avm)!: data_copy permutation#16531

Merged
IlyasRidhuan merged 1 commit intonextfrom
ir/08-14-fix_avm_data_copy_permutation
Aug 28, 2025
Merged

fix(avm)!: data_copy permutation#16531
IlyasRidhuan merged 1 commit intonextfrom
ir/08-14-fix_avm_data_copy_permutation

Conversation

@IlyasRidhuan
Copy link
Copy Markdown
Contributor

@IlyasRidhuan IlyasRidhuan commented Aug 22, 2025

Implements the permutations for cd_copy and rd_copy to the execution trace. It needs to be two different permutations (even though they lead to the same gadget) because they operate on different columns of execution (parent vs child). Hence, most instance of DATACOPY have been split into CDCOPY and RDCOPY

There was also a range of bug fixes uncovered because of this.

  1. Not including the parent calldata size at enqueued call contexts in execution.
  2. Not serialized parent calldata information in the when entering a new call

Calldata size needs to be properly handled at the tx trace level (in conjunction with calldata hashing). It's not solved here but tracked here

Copy link
Copy Markdown
Contributor Author

IlyasRidhuan commented Aug 22, 2025

Comment thread barretenberg/cpp/src/barretenberg/vm2/constraining/relations/data_copy.test.cpp Outdated
// Calldata info from parent context
uint32_t parent_cd_addr;
uint32_t parent_cd_size_addr;
uint32_t parent_cd_size;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not an address any longer, it's loaded into a register

.bytecode_id = parent_context.get_bytecode_manager().try_get_bytecode_id().value_or(FF(0)),
.is_static = parent_context.get_is_static(),
.parent_cd_addr = parent_context.get_parent_cd_addr(),
.parent_cd_size = parent_context.get_parent_cd_size(),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was missing

@IlyasRidhuan IlyasRidhuan force-pushed the ir/08-14-fix_avm_data_copy_permutation branch 2 times, most recently from cc1251a to 7b772e2 Compare August 24, 2025 20:22
@IlyasRidhuan IlyasRidhuan force-pushed the ir/08-14-fix_avm_data_copy_permutation branch 2 times, most recently from 0d923ca to d8e3a52 Compare August 25, 2025 09:24
pol commit parent_calldata_addr;
pol commit parent_calldata_size;

pol commit last_child_id;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constraints added in later PR

Copy link
Copy Markdown
Contributor

@dbanks12 dbanks12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment thread barretenberg/cpp/pil/vm2/data_copy.pil
@IlyasRidhuan IlyasRidhuan force-pushed the ir/08-14-fix_avm_data_copy_permutation branch from d8e3a52 to 456ebd1 Compare August 27, 2025 13:44
@IlyasRidhuan IlyasRidhuan added this pull request to the merge queue Aug 27, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 27, 2025
@IlyasRidhuan IlyasRidhuan added this pull request to the merge queue Aug 27, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 27, 2025
@IlyasRidhuan IlyasRidhuan force-pushed the ir/08-14-fix_avm_data_copy_permutation branch from 456ebd1 to 76b8c1e Compare August 28, 2025 08:16
@IlyasRidhuan IlyasRidhuan added this pull request to the merge queue Aug 28, 2025
github-merge-queue Bot pushed a commit that referenced this pull request Aug 28, 2025
Implements the permutations for cd_copy and rd_copy to the execution
trace. It needs to be two different permutations (even though they lead
to the same gadget) because they operate on different columns of
execution (parent vs child). Hence, most instance of `DATACOPY` have
been split into `CDCOPY` and `RDCOPY`

There was also a range of bug fixes uncovered because of this.
1. Not including the parent calldata size at enqueued call contexts in
execution.
2. Not serialized parent calldata information in the when entering a new
call

Calldata size needs to be properly handled at the tx trace level (in
conjunction with calldata hashing). It's not solved here but tracked
[here](#14666)
@IlyasRidhuan IlyasRidhuan removed this pull request from the merge queue due to a manual request Aug 28, 2025
@IlyasRidhuan IlyasRidhuan added this pull request to the merge queue Aug 28, 2025
Merged via the queue into next with commit 2d608ff Aug 28, 2025
15 checks passed
@IlyasRidhuan IlyasRidhuan deleted the ir/08-14-fix_avm_data_copy_permutation branch August 28, 2025 10:46
github-merge-queue Bot pushed a commit that referenced this pull request Aug 29, 2025
Adds the constraints for the `last_child_id` introduced in
#16531. It has
similar behaviour to other return data information stored in the context
ludamad pushed a commit that referenced this pull request Dec 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants